Conversation
Due to LLM limitations, LLM coding assistants frequently steamroller these and convert them into straight quotes. We have two choices: fight them at every step and keep reverting their changes, or just give up on fancy quotes. I think that realistically, we wouldn't do the first one, and would instead just keep introducing noise into diffs.
WalkthroughThis PR standardizes typographic characters across the codebase by replacing typographic apostrophes, quotation marks, and smart quotes with ASCII equivalents throughout documentation, comments, and string literals. No functional code, control flow, or public API changes are introduced. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow a single, highly repetitive pattern (typographic → ASCII punctuation normalization) across all affected files. While the volume of files is substantial, the uniformity of modifications and lack of behavioral changes minimize complexity. Review effort is primarily to confirm consistency of normalization across the codebase. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AblyChat/Rooms.swift (1)
281-283: Typo in log message: “Recieved” → “Received”.Minor text polish for logs.
Apply:
- logger.log(message: "Recieved request to fail room creation with error \(error)", level: .debug) + logger.log(message: "Received request to fail room creation with error \(error)", level: .debug)
🧹 Nitpick comments (2)
CONTRIBUTING.md (1)
107-107: Optional: clarify with underscore “testsOnly_”.To avoid ambiguity, consider “APIs marked testsOnly_”.
As per coding guidelines
Sources/AblyChat/Logging.swift (1)
113-114: Doc symbol likely incorrect: “DefaultInternalLogHandler” doesn’t exist.Replace with “DefaultInternalLogger” to match the type name used above.
-/// The logging backend used by ``DefaultInternalLogHandler`` if the user has not provided their own. Uses Swift's `Logger` type for logging. +/// The logging backend used by ``DefaultInternalLogger`` if the user has not provided their own. Uses Swift's `Logger` type for logging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
CONTRIBUTING.md(4 hunks)Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift(2 hunks)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift(1 hunks)Sources/AblyChat/ChatClient.swift(1 hunks)Sources/AblyChat/Connection.swift(1 hunks)Sources/AblyChat/ErrorInfo.swift(1 hunks)Sources/AblyChat/Errors.swift(1 hunks)Sources/AblyChat/JSONValue.swift(5 hunks)Sources/AblyChat/Logging.swift(3 hunks)Sources/AblyChat/Message.swift(1 hunks)Sources/AblyChat/RoomLifecycleManager.swift(8 hunks)Sources/AblyChat/Rooms.swift(3 hunks)Sources/AblyChat/SimpleClock.swift(1 hunks)Sources/AblyChat/SubscriptionAsyncSequence.swift(2 hunks)Sources/AblyChat/SubscriptionStorage.swift(2 hunks)Sources/BuildTool/BuildTool.swift(13 hunks)Sources/BuildTool/DestinationFetcher.swift(1 hunks)Sources/BuildTool/ProcessRunner.swift(1 hunks)Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift(12 hunks)Tests/AblyChatTests/DefaultRoomOccupancyTests.swift(2 hunks)Tests/AblyChatTests/DefaultRoomTests.swift(2 hunks)Tests/AblyChatTests/DefaultRoomsTests.swift(10 hunks)Tests/AblyChatTests/Helpers/SynchronizedAccess.swift(1 hunks)Tests/AblyChatTests/Helpers/TestLogger.swift(1 hunks)Tests/AblyChatTests/JSONValueTests.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRealtime.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
Sources/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Sources/**/*.swift: Prefer protocol-based design; expose SDK functionality via protocols and prefer opaque return types (some Protocol) over existential any Protocol
Isolate all mutable state to the main actor; mark stateful objects with @mainactor
Public API should use typed throws with ARTErrorInfo
Use InternalError internally and convert to ARTErrorInfo at public API boundaries
Test-only APIs in the library must be prefixed testsOnly_ and wrapped in #if DEBUG
Reference Chat SDK spec items in implementation comments (e.g., // @SPEC CHA-RL3g)
For public structs exposed by the API, provide an explicit public memberwise initializer
When using AsyncSequence operators in @mainactor contexts, mark operator closures as @sendable
For Task, CheckedContinuation, and AsyncThrowingStream (which don’t support typed errors), use Result and call .get() to bridge typed errors
Avoid Dictionary.mapValues for typed-throwing transformations; use ablyChat_mapValuesWithTypedThrow
When the compiler struggles with typed throws, explicitly annotate do throws(InternalError) blocks
Specify error types in closures, e.g., try items.map { jsonValue throws(InternalError) in … }
Files:
Sources/AblyChat/SubscriptionStorage.swiftSources/BuildTool/DestinationFetcher.swiftSources/AblyChat/Logging.swiftSources/AblyChat/JSONValue.swiftSources/BuildTool/ProcessRunner.swiftSources/AblyChat/Connection.swiftSources/AblyChat/SubscriptionAsyncSequence.swiftSources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swiftSources/AblyChat/RoomLifecycleManager.swiftSources/AblyChat/Rooms.swiftSources/AblyChat/Message.swiftSources/AblyChat/ErrorInfo.swiftSources/AblyChat/SimpleClock.swiftSources/AblyChat/Errors.swiftSources/AblyChat/ChatClient.swiftSources/BuildTool/BuildTool.swift
Tests/AblyChatTests/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Tests/AblyChatTests/**/*.swift: Place all tests under Tests/AblyChatTests
Use test attribution tags: @SPEC, @specOneOf(m/n), @specPartial, @specUntested, @specNotApplicable with appropriate spec IDs and explanations
For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function (until Xcode 16.3+)
Files:
Tests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/Helpers/SynchronizedAccess.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultRoomOccupancyTests.swiftTests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swiftTests/AblyChatTests/JSONValueTests.swiftTests/AblyChatTests/Helpers/TestLogger.swift
Tests/AblyChatTests/Mocks/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Place mock implementations under Tests/AblyChatTests/Mocks
Files:
Tests/AblyChatTests/Mocks/MockRealtime.swift
🧬 Code graph analysis (1)
Sources/AblyChat/Logging.swift (5)
Tests/AblyChatTests/Helpers/TestLogger.swift (1)
log(9-15)Tests/AblyChatTests/IntegrationTests.swift (2)
log(23-25)log(36-38)Tests/AblyChatTests/Mocks/MockLogHandler.swift (1)
log(6-8)Tests/AblyChatTests/DefaultInternalLoggerTests.swift (1)
log(12-29)Tests/AblyChatTests/Mocks/MockInternalLogger.swift (1)
log(6-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Example app, macOS (Xcode 26.0)
- GitHub Check: SPM (Xcode 26.0)
- GitHub Check: Example app, tvOS (Xcode 26.0)
- GitHub Check: Xcode, tvOS (Xcode 26.0)
- GitHub Check: SPM,
releaseconfiguration (Xcode 26.0) - GitHub Check: Example app, iOS (Xcode 26.0)
- GitHub Check: Xcode, macOS (Xcode 26.0)
- GitHub Check: Xcode, iOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 26.0) - GitHub Check: lint
🔇 Additional comments (57)
Tests/AblyChatTests/Helpers/TestLogger.swift (1)
4-4: LGTM: wording/quote normalization onlyNo functional changes.
Sources/AblyChat/SubscriptionStorage.swift (2)
36-36: LGTM: doc comment typo fixed“receiver's” spelling correction; code untouched.
77-77: LGTM: doc comment typo fixedConsistent with above; no behavior impact.
Tests/AblyChatTests/DefaultRoomOccupancyTests.swift (2)
55-55: LGTM: test comment normalizationNo effect on tests or spec coverage.
137-137: LGTM: test comment normalizationNo behavioral changes.
Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
5-5: LGTM: doc comment normalizationText-only change; safe.
Sources/BuildTool/ProcessRunner.swift (1)
7-7: LGTM: comment-only text normalizationThis is a straightforward comment edit with no behavioral impact. The repository-wide scan for smart quotes confirms no problematic occurrences in code comments.
Sources/AblyChat/SimpleClock.swift (1)
5-5: Doc punctuation normalization acknowledged.Purely textual; safe.
Tests/AblyChatTests/Helpers/SynchronizedAccess.swift (1)
3-3: Test helper comments: standardized quotes.No behavior change; good to go.
Also applies to: 5-5
Tests/AblyChatTests/DefaultRoomTests.swift (1)
212-213: Test comments standardized; intent preserved.Non-functional; safe.
Also applies to: 233-233
Sources/AblyChat/ChatClient.swift (1)
160-160: Docs: ASCII apostrophe replacement.No code changes; approve.
Sources/AblyChat/Connection.swift (1)
69-69: ---Comment-only change; standardization confirmed.
Verification confirms no curly quotes remain in Swift sources. The change is sound with no functional impact.
Optional: enforce quote style in SwiftLint to prevent regressions by adding to
.swiftlint.yml:custom_rules: disallow_curly_quotes: name: "Disallow Curly Quotes" regex: "[\u2018\u2019\u201C\u201D]" message: "Use straight ASCII quotes (' and \")." severity: errorTests/AblyChatTests/JSONValueTests.swift (1)
104-104: LGTM — comment normalization only.Sources/AblyChat/ErrorInfo.swift (1)
199-199: LGTM — documentation tweak, no behavior change.Sources/AblyChat/SubscriptionAsyncSequence.swift (1)
7-7: LGTM — comment-only normalization.Also applies to: 9-9, 20-20
CONTRIBUTING.md (2)
39-40: LGTM — prose only; quotes standardized.
138-138: LGTM — text normalization; no procedural changes.Also applies to: 142-142
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
4-4: LGTM: comment-only quote normalization.No behavioral changes; safe.
Also applies to: 62-62
Sources/AblyChat/Rooms.swift (2)
88-88: LGTM: documentation punctuation normalized.Text-only edits; API and behavior unchanged.
Also applies to: 220-220, 359-359
88-88: Review comment is incorrect; line 88 contains ASCII quotes, not curly quotes.The regex search for
[''""—…]found no matches in Sources/AblyChat/Rooms.swift. The visible snippet shows line 88 with ASCII straight quotes ("room map"), confirmed bycat -Aoutput. The suggestion to add a CI check is based on a false premise.Likely an incorrect or invalid review comment.
Sources/AblyChat/Errors.swift (1)
140-140: LGTM: docstring apostrophe normalized.No code or API impact.
Sources/AblyChat/Logging.swift (1)
52-52: LGTM: doc comments normalized.Text-only; no behavior change.
Also applies to: 71-71, 114-114
Sources/BuildTool/DestinationFetcher.swift (1)
14-14: No action required; code verified as correct.The error message at line 14 correctly uses an ASCII apostrophe in "Couldn't find a simulator". Verification found no tests asserting this exact message, so there are no test updates to make.
Sources/AblyChat/Message.swift (2)
224-229: Comment normalization only; LGTM.
Spec note preserved; no logic change.
224-229: The reviewed code contains no smart punctuation—disregard this review comment.The verification confirms that Sources/AblyChat/Message.swift lines 224-229 contain only standard ASCII characters, with no smart quotes, em dashes, or other Unicode punctuation. The code snippet shown in the review is clean. While the codebase does have smart punctuation elsewhere (confirmed in documentation and test files), the specific code under review is not affected and requires no changes.
Likely an incorrect or invalid review comment.
Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (10)
10-16: Docstring punctuation normalized; LGTM.
No code behavior changes.
87-89: Clarified “no‑op” wording; LGTM.
Expectation unchanged.
156-160: Comment clarity tweak; LGTM.
No test semantics altered.
190-192: Post-test note punctuation only; LGTM.
247-251: Bullet wording normalized; LGTM.
Assertions untouched.
272-274: “no‑op” phrasing normalized; LGTM.
358-361: Dash/quotes normalized; LGTM.
No timing/await logic changes.
391-393: Post-test note normalized; LGTM.
536-540: Comment punctuation only; LGTM.
572-574: Post-test note normalized; LGTM.Tests/AblyChatTests/DefaultRoomsTests.swift (10)
10-18: Docstring normalization; LGTM.
No behavioral impact.
110-113: Spec comment wording normalized; LGTM.
135-141: “Given” explanatory comments normalized; LGTM.
183-186: Spec-language comment normalization; LGTM.
206-213: “Given”/waiting explanation punctuation; LGTM.
283-286: “no‑op” phrasing normalized; LGTM.
324-327: Spec “future” wording normalized; LGTM.
353-356: Reference to spec “future” normalized; LGTM.
364-374: Error expectation comment clarified; LGTM.
380-383: Spec reuse wording normalized; LGTM.Sources/AblyChat/JSONValue.swift (5)
9-31: Top‑level docstring normalized; LGTM.
Examples and notes unchanged.
151-153: Bridging bullets reworded; LGTM.
No API change.
178-181: Extras doc normalized; LGTM.
Assumptions unchanged.
195-198: Usage bullets normalized; LGTM.
221-224: Dictionary bridging bullets normalized; LGTM.Sources/AblyChat/RoomLifecycleManager.swift (6)
299-301: Doc note punctuation normalized; LGTM.
346-351: Operation docs normalized (contractions/possessives); LGTM.
381-389: ATTACH operation doc header normalized; LGTM.
444-452: DETACH operation doc header normalized; LGTM.
509-517: RELEASE operation doc header normalized; LGTM.
587-590: Inline explanatory comments normalized; LGTM.Sources/BuildTool/BuildTool.swift (1)
117-117: All quote replacements are cosmetic and complete—no functional impact.The changes systematically replace typographic quotes (curly/smart quotes) with ASCII equivalents across documentation comments and string literals throughout the file. No functional code, control flow, declarations, or public API signatures were modified. This aligns cleanly with the PR objectives to standardize typographic characters and avoid ongoing diff noise from LLM conversions.
Also applies to: 153-153, 198-198, 223-223, 225-225, 302-302, 325-325, 326-326, 437-437, 613-613, 614-614, 615-615, 616-616, 617-617, 618-618, 678-678, 758-758, 763-763, 772-772, 800-800, 818-818
maratal
left a comment
maratal
left a comment
There was a problem hiding this comment.
I thought these were fancy and prepared to be upset, lol
2 participants
Note: This PR is based on top of #432.
Due to LLM limitations, LLM coding assistants frequently steamroller these and convert them into straight quotes. We have two choices: fight them at every step and keep reverting their changes, or just give up on fancy quotes. I think that realistically, we wouldn't do the first one, and would instead just keep introducing noise into diffs.
Summary by CodeRabbit
Style
Documentation